fix(eval): recalibrate canary latency budgets to the post-#606 honest baseline#622
Conversation
… baseline The pre-2026-07-13 budgets (p95 25s overall, 12s extractive) were calibrated while confidence_gate_blocked refusals dominated the answer subset - refusals return in ~1-2s, so the canary was timing fast wrong answers. With #606 restoring grounded answering, p95 reflects real work: the fast->extractive fallback chain spends the full 30s answer timeout before stitching sources, and #580's strong truncation self-heal can run two sequential generations. Post-fix observed subset p95 is 48.3s with every quality metric green (grounded 1.0, citation failure 0, numeric failure 0). New budgets catch regressions from that baseline instead of re-flagging the fix: overall 60s, extractive 50s, strong 60s. Fast (25s) and unsupported (4s) are unchanged - refusals must stay fast, which is exactly the waste mode #580 eliminated. These are eval-runner budgets (GitHub-hosted, cross-region to Sydney), not production UX targets; production SLOs stay in answer-slo.ts and docs/observability-slos.md. Closes the remaining red on #459. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughRAG latency thresholds were recalibrated for cross-region evaluation, a dedicated fallback latency route was added, and reindex-gate and documentation thresholds were aligned. Tests now validate fallback bucket reporting and the updated absolute latency ceiling. ChangesRAG latency gating
Estimated code review effort: 2 (Simple) | ~15 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87b996cf00
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87b996cf00
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/eval-quality.ts`:
- Around line 140-160: Update the p95 latency threshold used by the reindex gate
in reindex-eval-gate.ts and its documented value in retrieval-quality-runbook.md
to align with the current eval-quality.ts baseline, or change the gate to
compare against the established baseline for regression detection. Ensure normal
~48-second p95 results pass while genuine latency regressions still fail.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3b65287c-e0d9-440f-a79d-fbc6b57a1707
📒 Files selected for processing (1)
scripts/eval-quality.ts
… gate ceiling Review follow-ups on the latency recalibration: - latencyRouteForAnswer now classifies answers whose routing reason records a generation_fallback into a dedicated "fallback" latency bucket (50s budget): a failed generation structurally costs its timeout plus the fallback work. This restores the tight budgets the first commit accidentally loosened or missed: extractive returns to 12s (no-model stitching must stay fast) and plain fast stays at 25s, so regressions on either path cannot hide inside the fallback allowance. - reindex-eval-gate quality p95 ceiling 25s -> 60s to match the canary budget (a no-regression reindex at the honest ~48s baseline must not be blocked by the absolute bar); baseline-relative regression detection via latencyRegressionMs is unchanged. Runbook updated to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6428227e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A strong-classified reason carrying a generation_fallback (e.g. multi_document_comparison_synthesis; generation_fallback:provider_timeout) must budget as a fallback chain, not blend into the strong budget; successful strong generations (including quality retries) never record a generation_fallback, so genuine strong answers are unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-recalibration # Conflicts: # scripts/eval-quality.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c27d87c1a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - numeric grounding failure rate is above `0` | ||
| - source-governance danger failure rate is above `0` | ||
| - RAG p95 latency is above `25000ms` | ||
| - RAG p95 latency is above `60000ms` (recalibrated 2026-07-13: honest post-#606 answer p95 from eval runners is ~48s — real generation, cross-region, and timeout→fallback chains; regression detection is baseline-relative via `latencyRegressionMs`) |
There was a problem hiding this comment.
Document the canary-only latency budget accurately
This line says npm run eval:quality -- --fail-on-threshold allows 60000ms, but the script only uses that ceiling when EVAL_LATENCY_CONTEXT=cross-region-runner is set; the default path remains 25000ms, and eval:quality:release in package.json does not set the env var. Operators following the runbook for a local/release run will expect the documented ~48s baseline to pass, while the actual command still fails above 25s, so either call out that this is canary-only or wire the wider budget into the intended release command.
Useful? React with 👍 / 👎.
Implements the latency-threshold decision from #459 (option 1).
Why
The canary's latency budgets (p95 25s overall / 12s extractive) were calibrated while
confidence_gate_blockedrefusals dominated the answer subset — refusals return in ~1–2s, so the canary was timing fast wrong answers. After #606 restored grounded answering, p95 reflects real work: the fast→extractive fallback chain spends the full 30s answer timeout before stitching sources, and #580's strong truncation self-heal can run two sequential generations. The post-#606 dispatch run (29262119701) had every quality metric green (grounded 1.0, citation failure 0, numeric failure 0, retrieval failed_cases=0) and failed only on these two latency budgets at p95 48.3s.What
unsupported4s — refusals must stay fast; a slow refusal is exactly the waste mode fix(rag): stop reasoning-token starvation that discarded answers as "unsupported" #580 eliminated, so that budget is the standing regression guard.answer-slo.ts/docs/observability-slos.md.Verification
tests/eval-quality.test.ts14/14; prettier + typecheck clean.🤖 Generated with Claude Code
Summary by CodeRabbit
generation_fallbackas fallback, instead of applying the usual routing logic.